Skip to content

refactor(issue-fix): read the queue receipt version from its domain pack - #8183

Merged
huangruiteng merged 4 commits into
loopx-project:mainfrom
Jiaqiiii523:refactor/issue-fix-queue-receipt-version-owner
Nov 7, 2026
Merged

huangruiteng merged 4 commits into
loopx-project:mainfrom
Jiaqiiii523:refactor/issue-fix-queue-receipt-version-owner

Conversation

@Jiaqiiii523

@Jiaqiiii523 Jiaqiiii523 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Goal And Delivered Outcome

  • Outcome basis / optional anchor: self-contained maintenance outcome, no issue. The repository already rules on this exact pair: tests/architecture/test_issue_fix_local_time_owner.py records that "three modules stated it: the issue-fix domain pack plus two capability surfaces", and its CONSUMERS table lists loopx/capabilities/issue_fix/reviewer_notification.py as a module that must not keep a local binding. The queue receipt version is the second constant in the same two files that was never folded.
  • Goal/source and gap: issue_fix_reviewer_notification_queue_receipt_v1 was stated twice — loopx/domain_packs/issue_fix.py:21 (REVIEWER_NOTIFICATION_QUEUE_RECEIPT_SCHEMA_VERSION, the owner the domain pack validates against at :443) and loopx/capabilities/issue_fix/reviewer_notification.py:31 under an ISSUE_FIX_ prefix. A bump in one file would leave the other accepting the superseded contract, which is the drift the sibling owner test exists to prevent.
  • Observable before → after, with the validation row that proves it: before, the capability surface bound its own string; after, it binds the owner's object and the census over loopx/**/*.py reports exactly one definition. Proven by the two guard rows below plus the mutation check in Coverage and gaps.
  • Issue/task and intended base: no issue is closed. Intended base main.

Author Declaration

  • Written by: model_agent (Qoder Desktop coding agent, Anthropic Claude model family).

Implemented against

  • Specification and revision: the single-owner rule stated in tests/architecture/test_issue_fix_local_time_owner.py (docstring layers 1-3 and its CONSUMERS/DECLARED_INDIVIDUAL_SITES mechanism), applied at repository main 62e234c939996ad42f2b1168451cc9a656dce11d. No entry in loopx/semantics/vocabulary_v0.json registers this constant, so the fold does not move a registered owner.
Criterion (spec clause) Disposition Symbol / path Test or command
Layer 1 — judge folded values, not spellings implemented tests/architecture/test_issue_fix_queue_receipt_version_owner.py::test_only_the_domain_pack_states_the_queue_receipt_version python3 -m pytest -q tests/architecture/test_issue_fix_queue_receipt_version_owner.py
Layer 2 — pin the delegating module to the owner object, no local binding implemented ...::test_the_consumer_binds_the_owner_object_without_a_local_copy same command
Layer 3 — record the owner's verdict at the boundaries implemented ...::test_current_and_legacy_receipt_versions_stay_distinct_at_the_boundary same command
Declaration budget stays empty implemented DECLARED_UNFOLDABLE_SITES = {} same command
  • Self-check before submission: I read both definitions, the domain pack's own validation site, the sibling owner test, and the import block the consumer already used for the local-time shape. I kept the consumer's public name through an import ... as ... binding instead of renaming three call sites, because that is how the landed managed-marker fold handled the same situation. I checked that nothing outside the module imports the prefixed name, so no caller was renamed. I deliberately left the other ISSUE_FIX_REVIEWER_NOTIFICATION_*_SCHEMA_VERSION constants alone: each has exactly one definition today.

Scope And Continuation

  • Completed scope and remaining work: complete within this scope. The semantic inventory scanner reports closed-set vocabularies rather than single string constants, so no registry count moves here; widening the scanner to pin string families is a separate decision for a maintainer.
  • Slice boundary / successor: N/A — one value, one owner, independently reversible.

Validation

  • Tested revision: b6b09fb5e1e1611f0c9a58e6d240812997b9e6e5
  • Run state: finished
  • Input classes: public_fixture, synthetic
Check kind Result Public-safe evidence / limitation
unit passed python3 -m pytest -q tests/architecture/test_issue_fix_queue_receipt_version_owner.py tests/architecture/test_issue_fix_local_time_owner.py → 26 passed.
regression_parity passed Same interpreter and same six-file selection (tests/architecture/test_control_plane_import_boundaries.py, test_content_digest_single_owner.py, test_semantic_inventory.py, test_lark_identity_shape_owner.py, tests/capabilities/test_reward_memory_experiment.py, tests/capabilities/test_pr_review_contract.py), run twice on this host: clean base checkout of d0985c35aa3d05fd5de964e719aaf69f45d72535 → 3 failed, 472 passed; this head → the same 3 failed, 472 passed. Net regressions 0. The three pre-existing failures are test_turn_preview_import_does_not_select_report_delivery_or_lark_hosts, test_the_consumer_manifest_is_exactly_the_pinned_set and test_only_the_owner_module_states_a_whole_value_digest_shape, all of which reproduce on the untouched base.
static passed python3 -m ruff check on the two changed files.
integration not_run Not applicable: the value read by the receipt validator is unchanged, and no CLI or backend path is modified.
  • Coverage and gaps: the guard scans every tracked loopx/**/*.py for a module-level constant whose folded value equals the receipt version, so a re-added copy — written across lines, concatenated, or interpolated into an f-string — is reported. I poisoned the guard rather than assuming it bites: adding a second definition failed only layer 1, and changing the owner's value to another revision failed all three layers; both mutations were reverted and the restored file compared byte-identical. Gap named honestly: no shipped test drives a queued receipt end to end through this constant (grep over tests/ finds the value only in the new guard), so boundary behaviour rests on the validator in the domain pack; no frontend or real-backend path is touched.

Frontend / Visual Evidence

  • UI impact: none
  • Before: N/A
  • After: N/A
  • First viewport and preview: N/A
  • States and viewports shown: N/A
  • Source data: none
  • Attention review: N/A
  • Craft review: N/A

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Refactoring (no functional changes)
  • Documentation update
  • Test update

LoopX Area

  • Control plane (goals, todos, quota, scheduler, registry, runtime)
  • Benchmark boundary (adapters, runners, verifiers, scoring, evidence)
  • Capability or extension (providers, adapters, skills)
  • Public docs or presentation surface (README, protocols, dashboard)
  • Build, packaging, installer, or CI
  • Host or runtime integration

Technical Direction

  • Direction / acceptance reference, when applicable: Core control-plane hardening (single-owner vocabulary hygiene).

Shared-authority RFC fixture impact

  • Production-scale fixture schema: N/A
  • Semantic dimensions changed, or reviewed no-impact rationale: N/A — this constant is not a registered vocabulary in loopx/semantics/vocabulary_v0.json, and no RFC claims it.
  • Provider conformance arms run: N/A
  • Read-only legacy/file/PostgreSQL three-arm rehearsal: N/A

Boundary Checklist

  • Neither the diff nor this PR body/comments/attachments disclose private state, credentials, raw traces or verifier output, internal links, or local machine paths.
  • I did not duplicate maintainer-owned benchmark work unless a maintainer split out a public issue for it.
  • I kept the change scoped to the linked issue/task.
  • Existing English/Chinese RFC pairs both have a diff, or no paired RFC changed: no RFC file changed.
  • I completed the visual evidence section for UI changes, or marked UI impact none.
  • Every commit includes a DCO Signed-off-by trailer.

Rebase note: rebased onto d0985c35aa3d05fd5de964e719aaf69f45d72535 with no content change (git patch-id --stable identical: 69bc07982a79a5840364); the guard pair re-run on the rebased revision (26 passed) and the parity batch re-run against that base.

Second rebase in this session: moved onto d0985c35aa3d05fd5de964e719aaf69f45d72535 (main advanced twice while these were open). git patch-id --stable unchanged per branch (f3ce546bc69ef120722a, 69bc07982a79a5840364, cf8f91054e388d99a8b7), and the branch's own checks were re-run on that revision: the six smoke consumers exit 0 with the change and exit 1 without it, the guard pair is 26 passed against a 3 failed, 472 passed baseline identical on the untouched base, and the TypeScript lane is 6225 tests / 2 fail against 6224 / 2 fail with the same two failing test names, tsc --noEmit clean.

@Jiaqiiii523
Jiaqiiii523 force-pushed the refactor/issue-fix-queue-receipt-version-owner branch from 743d8c1 to 82ac4bd Compare November 6, 2026 14:39
Two modules stated issue_fix_reviewer_notification_queue_receipt_v1, and the
capability surface already imported the local-time shape from the same domain
pack. The guard follows the existing owner-test shape: census the folded value,
pin the consumer to the owner object, and keep current and legacy receipts
distinct.

Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com>
@Jiaqiiii523
Jiaqiiii523 force-pushed the refactor/issue-fix-queue-receipt-version-owner branch from 82ac4bd to b6b09fb Compare November 6, 2026 14:56

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=max

动机 / Motivation

维护 reviewer 通知队列协议的开发者,需要在同一处修改回执版本,避免生产者与持久化校验者接受不同版本。

原来 capability 与 domain pack 各写一份 v1 字符串;本头改为导入同一 owner 并保留原别名,版本变更可以集中维护,但新 guard 仍会放过拆开的重复定义。

生产导入折叠及当前队列行为已独立验证;防止重复定义回流的验收尚未满足,需修复已复现的 guard 漏检。

范围是一个既有版本常量及其回归检查,不改变发送授权、版本迁移、交付窗口、默认启用状态、CLI 参数或产品界面。

改动思路 / Approach

复用已导入的 domain pack,把 capability 原有公开别名绑定到该 owner;三个生产调用点不改名、不增加分支。这是合理的内部维护边界,当前 outcome-first/recovery 经验不应扩大成新 UI 或安装要求,也不提供历史结论或合并权限。

生产常量应由已存在的 domain pack 统一决定;测试本地 helper 只负责验证,源码子串不能代替其承诺的值判断。

当前边界是一个 import alias 与单 owner guard;最小修复限于 guard 的过滤与两个反例,不需要迁移产品 owner、增加 CLI 或新建跟踪任务。

规格在 diff 前读取:tests/architecture/test_issue_fix_local_time_owner.py,不可变版本 d0985c35aa3d05fd5de964e719aaf69f45d72535。Layer 1 的“求值而非拼写”验收未满足;Layer 2 的 owner binding 已验证;Layer 3 的生产边界通过独立队列和 CLI 检查。另对照既有通知协议保留 v1/v0、preview、交付窗口和迁移拒绝语义。

具体改动 / Changes

2 文件 +149/-4:生产代码 +5/-4,仅替换一个重复常量;新增144行 architecture test。主实现的 import alias 保持原前缀名字,仍被 queue reader、queued result builder 和结果 validator 使用;domain pack 持久化校验继续读取同一 v1。既有 v0 检测保留为迁移拒绝路径,没有增加旧版本兼容读取器。

P2:源码子串过滤使 folded-value guard 漏检

在 version_rows 的78–79行,源码不含连续 queue_receipt 就直接返回,尚未进入 AST 求值。例如:

COPY = "issue_fix_reviewer_notification_" + "queue_" + "receipt_v1"

该表达式求值后是完整当前版本,源码中却没有连续的过滤 token。另一个跨行相邻字符串案例同样漏检。独立临时仓库同时放入真实 owner 和这些副本后,module_constants 能求得正确值,但 collect 只返回 owner,完整 test_only_the_domain_pack_states_the_queue_receipt_version 仍通过。完整字面量副本作为阳性对照则正确失败。因此现有26项绿色测试不能证明本 PR 所声称的拼写无关防漂移边界。

移除按源码子串提前返回的过滤,在 AST 求值后判断版本值,并增加跨行相邻字符串及拆分加法的负例;两者都应使完整 owner census 测试失败。

生产路径讲解与验证

生产版本合并本身没有发现回归:实际 builder 在窗口外生成 v1,domain validator 接受并写入临时 JSONL;reader 保持去重并把 v0 与 v1 分开。基线与本头分别运行真实 loopx issue-fix reviewer-notification-drain 子进程:未配置返回 not_configured;未来 v1 返回 no_due_notifications、未到期计数1;v0 返回 reviewer_notification_queue_v1_migration_required 且 exit1。三种情况均不外读、不发送、不写状态,ledger 字节不变。除实际调用时钟字段外,完整 CLI 包与完整 producer 包逐项相等。

对主干的风险 / Risks

当前 wire value、默认开关、诊断与迁移行为保持;风险在长期维护 guard 的假绿,重复版本可能在后续 bump 后重新分叉。应修 guard,而非撤销有价值的 owner import 或放松版本拒绝。新测试并非通用权限 owner,不能因其扫描成功而推断发送授权。

独立执行 owner test pair 26 PASS;两个既有通知 smoke 在基线/本头均通过,覆盖窗口、preview、发送后校验失败和重启去重(provider 为合成实现)。上述3个实际 CLI 分支和 JSONL readback补充真实入口证据;这些结果没有把模拟通知说成真实飞书发送。Ruff、配置的 mypy17、whitespace 和两文件公开边界通过。原始 guard 反例为 FAIL;CQR cqr_4531fe008813ba986947 如实记录失败,merge gate 保持 hold。

作者报告的更大 batch 三项失败仅保留为未归因诊断,未用相同计数推断原因,未查询或等待 CI。最小回滚是撤回该 bounded commit;最小修复是上述过滤与负例,无数据迁移。未把内部常量维护扩展成新产品功能、第二语义 owner 或必需的测试重构工程。

我的整体评价 / Verdict

请求修改一个 P2:修复新 guard 的两个可复现漏检,并重新运行完整 guard 和当前队列路径验证。生产 owner 折叠方向与现有协议一致,但“以后不会以其他拼写重引入同一值”的承诺还没有成立。这个结论不指称当前线上队列损坏,也不把本次内部修复视作父级 Goal 或长程效果验收。

检查的两个改动路径仅包含 Python import/常量移除和 architecture test;没有渲染页面或用户文案变更,视觉审阅不适用。

没有验证实际飞书服务、Windows、安装分发或完整 CI;合成 provider 的通过不等于远端通知成功。

English verdict: REQUEST_CHANGES — HEAD b6b09fb. The production owner fold preserves current queue behavior, but the new guard misses split-literal duplicates; fix the prefilter and add decisive negative coverage.



def version_rows(source: str, name: str) -> list[dict[str, object]]:
if "queue_receipt" not in source:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 在 AST 求值后判断重复版本,避免源码子串漏检

其他模块用跨行相邻字符串或 + 拆开 queue_ 与 receipt_v1 来声明同一版本。 字面源码不含 queue_receipt 就提前返回,尽管求值等于完整 v1;重复版本进入仓库时完整 owner census 测试仍通过。 独立临时仓库中的完整 guard 复现了漏检;普通完整字面量对照能被拒绝。移除按源码子串提前返回的过滤,在 AST 求值后判断版本值,并增加跨行相邻字符串及拆分加法的负例;两者都应使完整 owner census 测试失败。

… source text

Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com>
@Jiaqiiii523

Copy link
Copy Markdown
Contributor Author

Fixed on a new head: f107a3d100046557360efcf4671349e2d79efc3e (1 file, +61/−2 — the architecture guard only; the production owner fold is untouched, as the review asked to keep).

The blind spot, reproduced before changing anything

Using this PR's own version_rows() / collect() / offenders() against a synthetic tree that holds the real owner file plus a planted copy:

Planted copy spelling Guard at b6b09fb5e1e Guard at f107a3d1000
contiguous full literal caught caught
"issue_fix_reviewer_notification_" + "queue_" + "receipt_v1" missed — census green caught, offenders = ["loopx/planted.py:1 restates the queue receipt schema version"]
adjacent literals across lines missed — census green caught
same-file constants concatenated missed — census green caught
adjacent f-string pieces missed — census green caught

The cause was exactly the two lines you pointed at: if "queue_receipt" not in source: return [] short-circuited before fold_string, while fold_string already folds BinOp Add, adjacent literals and same-file constants. So the module docstring's Layer 1 promise ("judges folded values rather than spellings") was contradicted by the prefilter, and the green census did not prove it.

Changes

  1. Removed the source-substring prefilter; the judgement is now the folded value alone.
  2. Added OFFENDING (4 spellings above) and NON_OFFENDING (the legacy _v0 receipt, an unrelated _queue_projection_v1 name) so the guard is shown to bite on copies and not to over-reach on different decisions.
  3. Added test_a_split_copy_is_rejected_through_the_repository_scan — a tmp_path tree where collect() + offenders() must fail the full census, which is the end-to-end version of the case you described.
  4. Shape follows tests/architecture/test_issue_fix_local_time_owner.py (OFFENDING / NON_OFFENDING / a tmp_path repository-scan probe) rather than inventing a second idiom.

Re-run on the new head

Check Result
pytest tests/architecture/test_issue_fix_queue_receipt_version_owner.py 10 passed (was 3)
control: prefilter re-inserted temporarily 5 failed — all four spellings plus the repository-scan case; file then restored byte-identical
pytest tests/architecture/test_issue_fix_*.py 33 passed
census over the real repository still passes, so removing the prefilter did not sweep up a legitimate site
examples/issue-fix-reviewer-notification-drain-postwrite-smoke.py, examples/issue-fix-reviewer-notification-sink-smoke.py both pass
ruff check on the changed file, git diff --check clean

Two limits I want on record rather than implied: I re-ran the two shipped notification smokes for the queue path, but not the three-branch reviewer-notification-drain subprocess matrix you described (unconfigured / future-v1 window / v0 migration) — that production behaviour is unchanged by this commit, so I am not claiming fresh evidence for it. And a f-string that interpolates a runtime value is still not foldable here (fold_string returns None for a JoinedStr with a FormattedValue); covering that shape needs the separate unfoldable-probe collector the sibling file has, which is out of scope for this P2. No hosted CI result is claimed for this head.

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent; gpt-6.1-sol; OpenAI; runtime_reported; reasoning_effort=max.

P2:带类型的同值常量绕过 owner 检查

version_rows 的第 84 行 和 module_constants 都只收 ast.Assign。在包含真实 owner 的临时树里加入 COPY: Final[str] = "issue_fix_reviewer_notification_queue_receipt_v1",完整 census 仍然通过。带类型的加法赋值、带类型的字符串操作数也一样;这三种值都能在运行前确定。普通字面量和原先漏掉的拆分写法作为对照,当前已被正确拒绝。

在现有测试内共用一个模块顶层赋值提取规则,识别有值的简单 AnnAssign,并把它用于常量求值和重复定义检查。用真实 owner 加带类型的副本跑完整 collect/census;当前 v0 和不同版本名称仍不应被判为副本。

动机 / Motivation

维护通知队列的开发者需要在一处修改回执版本,否则生成回执的一方和保存回执的一方可能接受不同版本。

这版把两个版本常量合并成一个,也修复了拆开字符串时的漏检;但用常见的类型注解声明同一常量,整仓检查仍会通过。

我确认当前生产链路保持原行为,原四种拆分副本已被拒绝;独立加入三个有类型的同值副本后,新检查仍未报告它们。

这次复审只覆盖既有队列版本和维护检查,不修改发送权限、默认开关、迁移策略或用户界面。

改动思路 / Approach

既有 issue-fix domain pack 继续拥有 v1 版本,capability 复用原别名;类型赋值漏检应在现有测试的同一提取规则内修复。 本 PR 的边界是一个版本来源折叠及能发现重复定义的检查,现有发送授权、迁移拒绝和交付窗口保持原样。

窗口外,现有 producer 用 domain pack 的 v1 值生成回执,原持久化校验把摘要、时间和去重键写入 JSONL;下一次 drain 读回该行,未到期保留,碰到 v0 仍要求手工迁移。只保留原来两份值会让后续改版有分叉风险;只测试导入对象也不能识别别处重声明。复用既有生产 owner,再补一个能被反例击中的测试,足以解决这个维护问题。没有理由为它添加新的兼容读取器或运行时扫描。

具体改动 / Changes

复审固定 8183@f107a3d100046557360efcf4671349e2d79efc3e,实际 base/merge-base d0985c3。完整两文件 +208/−4:生产 +5/−4,测试 +203。与上次审查的 b6b09fb5e1e1611f0c9a58e6d240812997b9e6e5 相比,只有测试 +61/−2;生产 alias、持久化 owner、现有 CLI/drain 源码不变。最新 main 的生产文件仍与不可变 base 相同,未发现已交付的替代。

用不可变 spec_revision=d0985c35aa3d05fd5de964e719aaf69f45d72535 的 tests/architecture/test_issue_fix_local_time_owner.py 三层职责和既有通知协议判断。Layer 1 未满足:注解常量仍漏检。Layer 2 通过:原消费者别名绑定既有 domain pack,删除一处本地声明。Layer 3 通过:实际文件队列、当前 v1 读回与旧 v0 的拒绝行为保持。

关键代码讲解

  • _queued_result 从同一版本值生成窗口外的回执,保留摘要、时间和去重键,不提前发送。
  • _validated_reviewer_notification_queue_receipt 继续由 domain pack 校验写入;不会接受缺少现有必填字段的旧版回执。
  • reviewer_notification_queue_from_state 继续按同一 schema 和 hash 读回,旧 v0 仍由单独的拒绝路径识别。
  • version_rows 去掉上一版的源码子串过滤后,四种原拆分副本不再漏过;但只遍历普通赋值,所以 AnnAssign 仍未进入求值。
  • module_constants 给字符串拼接提供同文件常量,带类型的操作数也在这里被漏掉。两处共用一条简单赋值提取规则即可。

对主干的风险 / Risks

独立把同一组 10 个夹具送进完整 census:当前已拒绝原四种拆分同值写法,普通同值对照也被拒绝,v0 和无关版本名正确通过;三个带注解的同值副本仍全部通过。因此 33 项通过不足以证明这项新增维护承诺。修复应在现有测试补 AnnAssign,覆盖同值字面量、拼接和操作数,再运行 uv run --extra test python -m pytest -q tests/architecture/test_issue_fix_queue_receipt_version_owner.py。无需对运行时动态表达式求值。

当前生产路径在不可变 base/head 独立运行:实际 builder 生成同一 v1,真实 JSONL 持久化读回及同键重试的完整包相同。实际 issue-fix reviewer-notification-drain 对关闭配置(已有 v0)、未到期 v1、v0/v1 混合、坏时间戳四种输入,得到同样的 not_configured、no_due_notifications 和两个明确拒绝;退出码、完整诊断、零 provider 调用和未修改的 ledger 字节一致,只排除实际调用时钟字段。两项现有通知 smoke 在两侧通过,provider 为合成边界;架构模块当前 33 PASS、base 23 PASS。

语义与 CI 对齐

现有 wire value、默认关闭、迁移拒绝与发送权限保持;没有新增语义词汇。仓库 Ruff 范围与改动文件 Ruff、Mypy 17 入口通过;补锁定的 npm dev 依赖后,quick premerge 6 个直接 / 3 个选择检查通过。严格 CQR cqr_8d1473af2873341cf965 仍 FAIL,原因就是当前 P2,机械检查通过不替代它。未查询或等待远端 CI;作者较早较大批次的失败保持其原版本和未独立归因状态。

我的整体评价 / Verdict

请求修改一项 P2。版本收拢的生产实现值得保留,既有重试/恢复及使用体验在已测范围内保持;新检查仍会对常见的带类型声明给出假绿,单一来源的维护验收还差这一步。修复限于现有 guard,重跑完整 census 和既有架构测试,不要求重写生产或重新迁移数据。

已检查现有 CLI 与通知结果;两处改动不涉及渲染页面、布局或首屏。没有运行活动飞书服务、安装版本或跨平台界面。当前结论不把合成 provider 的通过当成真实发送,也不改变合并权限或移除原讨论。

English verdict: REQUEST_CHANGES; f107a3d; typed, statically foldable duplicate declarations still pass the complete owner census. The original split-literal defect is fixed and the real queue paths retain parity.

constants = module_constants(tree)
rows: list[dict[str, object]] = []
for node in tree.body:
if not isinstance(node, ast.Assign) or len(node.targets) != 1:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] 带类型的同值常量绕过 owner 检查。COPY: Final[str] = "issue_fix_reviewer_notification_queue_receipt_v1" 在包含真实 owner 的临时树中通过完整 census;带注解的加法和操作数同样漏检。这里和 module_constants 都只收 ast.Assign。请在现有测试共用简单赋值提取,纳入有值的 AnnAssign,再用完整 collect/census 验证三例应拒绝。原拆分常量修复和生产 alias 可以保留。

…census

Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com>
@Jiaqiiii523

Copy link
Copy Markdown
Contributor Author

Fixed on a new head: c7280b3579c2d6f1d1290ec1126b303f0bbc7ce6 (1 file, +87/−17 over f107a3d1000; the production owner fold is still untouched).

Reproduced before changing anything

Using this PR's own version_rows() / collect() / offenders():

Planted copy head f107a3d1000 head c7280b3579c
COPY: Final[str] = "issue_fix_reviewer_notification_queue_receipt_v1" missed (0 rows) 1 row
COPY: str = "issue_fix_reviewer_notification_" + "queue_" + "receipt_v1" missed 1 row
HEAD: Final[str] = "…queue_" + COPY: Final[str] = HEAD + "receipt_v1" missed 1 row
full census, real owner + typed copy in one tree offenders = [] → the test is green offenders non-empty
typed legacy …_v0 / typed unrelated …_projection_v1 not flagged still not flagged

Exactly your diagnosis: module_constants and version_rows each walked only ast.Assign, so a typed binding never reached fold_string, even though its value is decided before anything runs.

The change

One shared extraction rule, stated_bindings(tree), that yields (name, lineno, value) for both a single-target ast.Assign and an ast.AnnAssign carrying a value. module_constants() now seeds its fold from it, and version_rows() now walks it for the duplicate scan — so constant folding and the restatement census read the same shape instead of two hand-written loops that can drift apart, which was the point of your note.

Coverage added, following the sibling file's idiom:

  • 3 more OFFENDING spellings (typed full literal, typed addition, typed constant operand);
  • 1 more NON_OFFENDING (typed legacy _v0) so the guard is shown not to over-reach;
  • test_the_census_flags_a_typed_copy_beside_the_real_owner — the repository-level case you asked for: the real owner file plus a typed copy in one tmp_path tree, asserting both rows are found and offenders() is non-empty;
  • test_the_census_leaves_a_typed_legacy_receipt_alone — collect() returns [] for a typed v0.

The module docstring's Layer 1 sentence now names the annotated form, so the promise and the implementation say the same thing.

Verification

Check Result
pytest -q tests/architecture/test_issue_fix_queue_receipt_version_owner.py 16 passed (10 before this commit, 3 at the original head)
control: ast.AnnAssign branch turned back into continue 4 failed — precisely the three typed spellings and the owner-plus-typed-copy census case; file then restored and re-run clean
pytest -q tests/architecture/test_issue_fix_*.py 39 passed (was 33)
census over the real repository still reports only the owner, so widening the extraction surfaced no existing restatement — this is a forward-looking guard, not a debt sweep
ruff check on the file, git diff --check clean

Limits on record: values that genuinely need runtime evaluation are still not folded (a JoinedStr with a real FormattedValue, or a call) — that would need the separate unfoldable-probe collector the sibling file carries, and you said no runtime evaluation is required here. No hosted CI result is claimed for this head.

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=max

P2 需要修复。 独立执行确认插值生成现有 v1。在保留真实 owner 的临时目录中加入副本,完整扫描仍通过。以后版本升级时,这种定义可与 owner 分叉。

动机 / Motivation

维护 issue-fix reviewer 通知的开发者需要在一个地方更新队列回执版本。通知稍后从已保存的回执恢复,版本分叉会影响读取。

队列写入方曾保留一份 v1 字符串,持久化校验方持有另一份。改动让两边共用一个值,并加上扫描以捕获未来的重复定义。

新提交拦住了带注解的字面量、拼接及同文件常量副本。两种静态 f-string 插值仍通过完整扫描,实际队列生成、持久化和三个源码 CLI 分支与基线一致。

评审范围是现有 v1 回执的常量归属和防漂移测试。实际执行覆盖源码 CLI、临时 JSONL 和合成通知 provider。

改动思路 / Approach

生产代码沿用既有 domain pack 常量。折叠逻辑适合留在架构测试,修复只需覆盖测试承诺的静态 f-string 分支。 当前 PR 的边界是一个生产导入别名和现有版本的单一 owner 检查。

两个历史问题已复验。初始版本的文本预筛会漏掉拆分字符串。上一头仍漏掉带值的类型注解。现在常量表和扫描共用 stated_bindings,完整扫描覆盖了这两种缺口。

具体改动 / Changes

全量差异为两文件 +278/-4。相对上一头仅测试变化 +87/-17。预变更规格为 tests/architecture/test_issue_fix_local_time_owner.py,版本 d0985c35aa3d05fd5de964e719aaf69f45d72535。Layer 1=not_met、Layer 2=implemented、Layer 3=implemented。 预变更规格

P2 · 静态 f-string 插值绕过队列版本单一 owner 检查

另一个模块以同文件静态字符串 PREFIX 构造 COPY: str = f"{PREFIX}receipt_v1",得到当前 v1 回执版本。 collect → version_rows → fold_string 的 ast.JoinedStr 分支丢弃 ast.FormattedValue,长度检查返回 None。 独立执行确认插值生成现有 v1。在保留真实 owner 的临时目录中加入副本,完整扫描仍通过。以后版本升级时,这种定义可与 owner 分叉。 等价的带注解加法副本会被拒绝,插值构造的 v0 会通过。让现有 fold_string 解析可静态求值的简单插值,并给完整扫描加上同文件常量插值和 v0 对照。 fold_string

PREFIX: str = "issue_fix_reviewer_notification_queue_"
COPY: str = f"{PREFIX}receipt_v1"

关键代码讲解

  • 消费方直接导入既有 owner 的 v1,排队结果和后续读取共用同一个值。 生产别名
  • 带值的 Assign 和 AnnAssign 同时进入常量收集与扫描。仅有注解的声明跳过。 stated_bindings
  • 扫描按折叠值判断,完整扫描在两种真实插值反例中只看到 domain owner。 version_rows

对主干的风险 / Risks

39 项针对性测试通过。基线与当前头生成相同回执,实际源码 CLI 的未配置、未来 v1、旧 v0 分支也一致。比较完整输出时只去掉调用时钟字段。CLI 未调用通知 provider,临时队列文件字节一致。两项仓库通知 smoke 在两版均通过。

Ruff 和配置的 Mypy 均通过。quick premerge 的 6 个直接检查和 3 个选择检查通过。相邻六文件两版各 472 项通过,完整诊断有相同的 3 项既有失败,涉及 Turn 预览导入、digest owner 和 issue-fix CLI manifest。严格质量回执 cqr_b0e4c958089621e4ff6f 因本次插值反例失败。

语义与 CI 对齐

现有可选配置、队列 v1 校验和 v0 迁移拒绝均走原入口。新增变化集中在架构测试的防漂移能力。托管 CI 按本轮策略未查询。

我的整体评价 / Verdict

当前头 c7280b3579c2d6f1d1290ec1126b303f0bbc7ce6 已修复前两轮反例。由于新增检查仍会漏掉承诺覆盖的简单静态插值,请修复后重跑完整扫描。改动路径是通知队列源码 CLI 和架构测试,已比较 CLI 输出和退出码,视觉审阅不适用。 真实飞书发送、Windows、安装分发及托管 CI 未验证。通知 smoke 使用合成 provider。

English verdict: REQUEST_CHANGES — The prior split and annotated assignment cases now pass the intended checks, and production behavior matches the base. Static f-string interpolation can still bypass the promised single-owner guard.

parts = [
value.value
for value in node.values
if isinstance(value, ast.Constant) and isinstance(value.value, str)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 · 静态 f-string 插值绕过队列版本单一 owner 检查

另一个模块以同文件静态字符串 PREFIX 构造 COPY: str = f"{PREFIX}receipt_v1",得到当前 v1 回执版本。 collect → version_rows → fold_string 的 ast.JoinedStr 分支丢弃 ast.FormattedValue,长度检查返回 None。 独立执行确认插值生成现有 v1。在保留真实 owner 的临时目录中加入副本,完整扫描仍通过。以后版本升级时,这种定义可与 owner 分叉。

让现有 fold_string 解析可静态求值的简单插值,并给完整扫描加上同文件常量插值和 v0 对照。

…e receipt guard

Signed-off-by: Jiaqiiii523 <293262669+Jiaqiiii523@users.noreply.github.com>
@Jiaqiiii523

Copy link
Copy Markdown
Contributor Author

Fixed on a new head: 2be88a04981fa78aa7c875dfb658fe87e686185e (1 file, +81/−8 over c7280b3579c; production still untouched).

Reproduced before changing anything

With this PR's own version_rows() at c7280b3579c:

PREFIX: str = "issue_fix_reviewer_notification_queue_"
COPY: str = f"{PREFIX}receipt_v1"

→ version_rows returned 0 rows; and the full census over a tree holding the real owner plus that copy reported offenders = [], i.e. green. Exactly your diagnosis: the JoinedStr branch only accepted Constant pieces, so a FormattedValue made the length check fail and the whole fold returned None — the value is decided before anything runs, but the guard never read it.

The change

fold_string now walks an f-string piece by piece instead of filtering by node type:

  • a Constant piece contributes its text;
  • a FormattedValue with no conversion and no format spec contributes the result of folding its inner expression (so a same-file Name resolves through the constant table, and nested folds keep the existing depth bound);
  • anything else — a non-string constant, a !r/!s conversion, a format spec, or an inner expression that cannot be decided statically — returns None.

So the fold widens only over spellings whose value is provably the same string, and stays silent about ones where the runtime text could differ.

Cases added, in the same shapes you named:

  • OFFENDING: interpolated same-file constant, and a typed annotation over an interpolated constant;
  • NON_OFFENDING: an interpolated legacy _v0 (different decision), plus a !r conversion and a :>60 format spec — these two pin the boundary so the new fold cannot be accused of matching text it should not;
  • test_the_census_flags_an_interpolated_copy_beside_the_real_owner — real owner file + interpolated copy in one tmp_path tree, asserting both rows and a non-empty offenders();
  • test_the_census_leaves_an_interpolated_legacy_receipt_alone — collect() returns [].

The module docstring's Layer 1 sentence now names interpolation from same-file constants.

Verification

Check Result
pytest -q tests/architecture/test_issue_fix_queue_receipt_version_owner.py 23 passed (16 before this commit; 3 at the original head)
control: JoinedStr branch restored to the old constant-only comprehension 3 failed — the two interpolation spellings plus the owner-plus-interpolated-copy census case; file restored and re-run clean
pytest -q tests/architecture/test_issue_fix_*.py 46 passed (was 39)
census over the real repository still reports only the owner, so widening the fold surfaced no existing restatement
ruff check on the file clean

Limits on record: an interpolation whose inner expression needs runtime evaluation (a call, an attribute, an index) is still None, by design — that is not a statically decidable spelling; and no hosted CI result is claimed for this head. The three failures the review listed as pre-existing are untouched here.

@loopx-agent loopx-agent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewer: model_agent | gpt-6.1-sol | OpenAI | runtime_reported | reasoning_effort=max

没有阻塞本次代码评审的问题。复审对应提交 2be88a0。上轮三个发现涉及拆分、带注解和静态插值副本。独立检查已在当前提交验证这些场景。

动机 / Motivation

维护审核通知队列的开发者要在一个地方更新回执版本。延迟发送会从保存的回执重新读取版本。 此前队列生成方和持久校验方各有一份 v1。架构检查在上一个提交还会漏掉由同文件字符串插值得到的副本。 当前提交引用同一个版本,并使四种简单插值副本在完整扫描中失败。真实 CLI 的现行 v1、旧 v0 和格式错误行为与基线一致。 交付范围是通知回执版本归属和测试覆盖。通知发送权限和版本迁移流程沿用现有规则。 相邻测试在两侧都有三项既有失败,严格质量回执仍未通过。真实外部发送与安装环境另行验收。

改动思路 / Approach

生产方只需要引用现有常量。新增判断留在架构测试,可以覆盖已复现的插值漏检。 本次边界是一个生产导入别名和针对当前队列版本的静态检查。 现有规则只折叠可静态解析的字符串。无转换和格式说明的插值可参与,旧 v0、repr、带填充的值和不相关声明仍与当前 v1 区分。这样生产者和持久校验方可以共用原有值,架构扫描也能识别真实的重复定义。

具体改动 / Changes

本次完整差异包含两个文件。生产方用现有 domain pack 的 v1 取代本地重复字面量,原有队列生成和重试继续使用该值。持久校验方维持原 v1 验证,旧回执仍进入已有的迁移拒绝路径。 队列生产方 校验方

以 d0985c3 的 owner 测试作为规格。Layer 1 的完整扫描拒绝简单静态插值构造的重复版本。Layer 2 检查生产消费方引用同一个版本。Layer 3 验证当前与旧回执仍在真实读取入口区分。 既有规格 插值与扫描

对主干的风险 / Risks

用 Python 的实际求值独立对照 14 种声明。上一头漏掉四种能求出同一版本值的简单插值,当前头全部符合预期。源码 CLI 的未配置、未来 v1、混合 v1/v0 和无效时间四组结果与基线一致,原队列文件字节一致,也没有调用外部 provider。两项通知 smoke 在两侧均通过。

46 项针对性测试、全量 Ruff、配置的 Mypy,以及 quick premerge 的六项直接和三项选中检查通过。相邻六文件测试在两侧均为 472 通过和三项相同失败,涉及 Turn 预览导入和 digest 检查,严格 CQR 因此仍失败。按策略未查询 CI。

我的整体评价 / Verdict

本次边界是一个生产导入别名和针对当前队列版本的静态检查。 现有回执的排队、重试和迁移拒绝分支具备独立的实测证据。改动集中在通知队列 CLI 与架构测试,视觉审阅不适用。 保留相邻测试失败和严格资格缺口,合并须另行核对。

English verdict: APPROVE. The existing queue version now has one source, the previously missed simple interpolations are rejected, and native receipt behavior matches the base. The three reproduced adjacent failures remain a separate strict qualification hold.

@huangruiteng
huangruiteng merged commit 9892d5e into loopx-project:main Nov 7, 2026
23 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants